Fix/last seen at by environment #4939

FredrikOseberg · 2023-10-05T12:52:55Z

This is a draft of the new architecture for last seen at. Apologies for size, but I initially explored triggers and couldn't merge that before it was done. I changed my approach mid-way and now I'd like to verify the direction and merge before doing the remaining endpoints.

Discussion

Today there are 5 pages accessing this information:

Project health
Project overview (DONE)
Feature overview
Feature toggle list
Archived features

The last seen at property is in it's current form being added to the data structure of the environments property that exists on the feature data structure on some of these views. Since some of the views don't have it, its implementation varies across views. I have chosen not to change the data structure in this iteration, which has influenced the design decisions of reading the data.

It might it would be better if the data was added as an additional field directly on the feature data structure itself, only for the read operations in the admin panel:

{ name: 'my-feature ..., lastSeenAtPerEnvironment: [ { 'development': { 'lastSeenAt': '2023-10-03 13:08:16.263,'  } } ] }

Other discussion points:

Went with batch inserts instead of triggers. Reasons:
- Can be feature flagged
- Triggers run on every row, and application will exponentially increase the amount of rows we need to insert LastSeenAt because it's a dimension we don't care about in the new table
- Would have to shut down releasing to production while testing trigger performance
Foreign key on environments to maintain data integrity when deleting environment

Follow up actions

Implement remaining endpoints
Increase test coverage
Land on data structure
Test with load
Add job to clean up (IE: Feature flags that do not exist)

vercel · 2023-10-05T12:53:02Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
unleash-docs	❌ Failed (Inspect)			Oct 9, 2023 7:26am
unleash-monorepo-frontend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 9, 2023 7:26am

FredrikOseberg · 2023-10-05T12:55:02Z

src/lib/services/client-metrics/last-seen/createLastSeenService.ts

+import { LastSeenService } from './last-seen-service';
+import LastSeenStore from './last-seen-store';
+
+export const createLastSeenService = (


Use composite root pattern for LastSeenService

FredrikOseberg · 2023-10-05T12:55:47Z

src/lib/services/client-metrics/last-seen/last-seen-mapper.ts

+import { IFeatureLastSeenResults } from './last-seen-read-model';
+
+export class LastSeenMapper {
+    mapToFeatures(


Map results of reading into environments of incoming features to maintain todays data structure, might change in follow up iteration

FredrikOseberg · 2023-10-05T12:56:23Z

src/lib/services/client-metrics/last-seen/last-seen-read-model.ts

+        };
+    };
+}
+export class LastSeenAtReadModel implements ILastSeenReadModel {


retrieve last seen at results per environment for an array of features

FredrikOseberg · 2023-10-05T12:56:42Z

src/lib/services/client-metrics/last-seen/last-seen-service.ts

+            if (this.config.flagResolver.isEnabled('useLastSeenRefactor')) {
+                await this.lastSeenStore.setLastSeen(lastSeenToggles);
+            } else {
+                await this.featureToggleStore.setLastSeen(lastSeenToggles);
+            }


maintain previous implementation

FredrikOseberg · 2023-10-05T12:57:02Z

src/lib/services/client-metrics/last-seen/last-seen-store.ts

+    async setLastSeen(data: LastSeenInput[]): Promise<void> {
+        const now = new Date();
+
+        try {
+            const inserts = data.map((item) => {
+                return {
+                    feature_name: item.featureName,
+                    environment: item.environment,
+                    last_seen_at: now,
+                };
+            });
+
+            const batchSize = 1000;
+
+            await this.db.transaction(async (trx) => {
+                for (let i = 0; i < inserts.length; i += batchSize) {
+                    const batch = inserts.slice(i, i + batchSize);
+                    // Knex optimizes multi row insert when given an array:
+                    // https://knexjs.org/guide/query-builder.html#insert
+                    await trx(TABLE)
+                        .insert(batch)
+                        .onConflict(['feature_name', 'environment'])
+                        .merge();
+                }
+            });
+        } catch (err) {
+            this.logger.error('Could not update lastSeen, error: ', err);
+        }


Batch insert last seen in batches of 1000

FredrikOseberg · 2023-10-05T12:59:53Z

src/lib/services/project-service.ts

+        if (this.flagResolver.isEnabled('useLastSeenRefactor')) {
+            const mapper = new LastSeenMapper();
+
+            const featureNames = features.map((feature) => feature.name);
+            const lastSeenAtPerEnvironment =
+                await this.lastSeenReadModel.getForFeature(featureNames);
+
+            console.log(decoratedFeatures, lastSeenAtPerEnvironment);
+
+            decoratedFeatures = mapper.mapToFeatures(
+                decoratedFeatures,
+                lastSeenAtPerEnvironment,
+                this.logger,
+            );
+        }


Decorate features with lastSeenData

FredrikOseberg · 2023-10-05T13:00:07Z

src/migrations/20231003113443-last-seen-at-metrics-table.js

+exports.up = function (db, callback) {
+    db.runSql(
+        `
+      CREATE TABLE last_seen_at_metrics (


new table, foreign key on environments

andreas-unleash · 2023-10-05T13:08:26Z

Looks good. Only suggestion is we could add the batch size as a variant in the flag. Design decisions make sense to me

kwasniew · 2023-10-05T14:17:21Z

src/lib/services/client-metrics/last-seen/last-seen-store.ts

+
+            const batchSize = 1000;
+
+            await this.db.transaction(async (trx) => {


I'd do the transaction around the service (in the controller) as we do for all new code.

Since it's a non critical data I'm wondering if having transactions even makes sense here. I'd consider sacrificing correctness for performance.

Agree, done

kwasniew · 2023-10-05T14:36:30Z

src/lib/services/client-metrics/last-seen/last-seen-service.ts

        this.featureToggleStore = featureToggleStore;
        this.logger = config.getLogger(
            '/services/client-metrics/last-seen-service.ts',
        );
+        this.config = config;

        this.timers.push(
            setInterval(() => this.store(), lastSeenInterval).unref(),


shouldn't this be part of the scheduler?

I will look into separating this in a follow-up

kwasniew · 2023-10-06T11:43:57Z

src/migrations/20231003113443-last-seen-at-metrics-table.js

+};
+
+exports.down = function (db, callback) {
+    callback();


missing down migration

kwasniew

I'd run clinic.js with both last seen variants to compare the performance characteristics

github-actions bot assigned FredrikOseberg Oct 5, 2023

FredrikOseberg commented Oct 5, 2023

View reviewed changes

FredrikOseberg requested review from sjaanus, kwasniew, ivarconr and chriswk October 5, 2023 13:00

vercel bot deployed to Preview – unleash-docs October 5, 2023 14:15 View deployment

vercel bot deployed to Preview – unleash-monorepo-frontend October 5, 2023 14:16 View deployment

kwasniew reviewed Oct 5, 2023

View reviewed changes

FredrikOseberg added 8 commits October 6, 2023 10:23

feat: last seen initial steps for new architecture

960a6d1

feat: map last seen to environments

5e95ffc

fix: add feature flag

93d4ffd

fix: restore test

1e65a1a

feat: test mapper

03c8364

feat: add foreign key

3d4991d

fix: remove foreign key on features

0fe9c75

fix: console.log

9a961a7

FredrikOseberg force-pushed the fix/last-seen-at-by-environment branch from b5c94c9 to 9a961a7 Compare October 6, 2023 08:28

fix: formatting

d93d6a5

vercel bot deployed to Preview – unleash-monorepo-frontend October 6, 2023 08:41 View deployment

fix: migration

766d1e3

vercel bot deployed to Preview – unleash-monorepo-frontend October 6, 2023 10:56 View deployment

vercel bot had a problem deploying to Preview – unleash-docs October 6, 2023 11:01 Failure

FredrikOseberg marked this pull request as ready for review October 6, 2023 11:28

fix: snapshot

83ece07

vercel bot deployed to Preview – unleash-monorepo-frontend October 6, 2023 11:38 View deployment

vercel bot had a problem deploying to Preview – unleash-docs October 6, 2023 11:42 Failure

kwasniew reviewed Oct 6, 2023

View reviewed changes

kwasniew approved these changes Oct 6, 2023

View reviewed changes

fix: down migration

2407520

vercel bot had a problem deploying to Preview – unleash-docs October 6, 2023 11:55 Failure

vercel bot deployed to Preview – unleash-monorepo-frontend October 6, 2023 11:56 View deployment

fix: tests

8742f5c

vercel bot deployed to Preview – unleash-monorepo-frontend October 6, 2023 12:53 View deployment

fix: add raw database

7d14af5

vercel bot had a problem deploying to Preview – unleash-docs October 6, 2023 13:05 Failure

vercel bot deployed to Preview – unleash-monorepo-frontend October 6, 2023 13:06 View deployment

fix: tests

98f5629

vercel bot deployed to Preview – unleash-monorepo-frontend October 9, 2023 07:07 View deployment

vercel bot had a problem deploying to Preview – unleash-docs October 9, 2023 07:12 Failure

fix: move test

a474998

vercel bot deployed to Preview – unleash-monorepo-frontend October 9, 2023 07:20 View deployment

vercel bot had a problem deploying to Preview – unleash-docs October 9, 2023 07:26 Failure

FredrikOseberg merged commit d896dbd into main Oct 9, 2023
14 checks passed

FredrikOseberg deleted the fix/last-seen-at-by-environment branch October 9, 2023 08:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/last seen at by environment #4939

Fix/last seen at by environment #4939

FredrikOseberg commented Oct 5, 2023 •

edited

Loading

vercel bot commented Oct 5, 2023 •

edited

Loading

FredrikOseberg Oct 5, 2023

FredrikOseberg Oct 5, 2023

FredrikOseberg Oct 5, 2023

FredrikOseberg Oct 5, 2023

FredrikOseberg Oct 5, 2023

FredrikOseberg Oct 5, 2023

FredrikOseberg Oct 5, 2023

andreas-unleash commented Oct 5, 2023

kwasniew Oct 5, 2023

kwasniew Oct 5, 2023

FredrikOseberg Oct 6, 2023

kwasniew Oct 5, 2023

FredrikOseberg Oct 6, 2023

kwasniew Oct 6, 2023

kwasniew left a comment


		const batchSize = 1000;

		await this.db.transaction(async (trx) => {

Fix/last seen at by environment #4939

Fix/last seen at by environment #4939

Conversation

FredrikOseberg commented Oct 5, 2023 • edited Loading

Discussion

vercel bot commented Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreas-unleash commented Oct 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwasniew left a comment

Choose a reason for hiding this comment

FredrikOseberg commented Oct 5, 2023 •

edited

Loading

vercel bot commented Oct 5, 2023 •

edited

Loading